-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: revoke recurring grants #1446
Conversation
# Conflicts: # packages/boutique/backend/src/order/controller.ts
# Conflicts: # packages/wallet/backend/src/grant/service.ts
@@ -222,10 +222,11 @@ export class OrderController implements IOrderController { | |||
res: TypedResponse<CreateResponse>, | |||
next: NextFunction | |||
) => { | |||
let order: Order | |||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I get this right, but is this just a delete of the orderService in case the creation did not work due to having the OP client unauthorized?
If that is the case, Couldn't this just be a transaction that is rolled back, instead of a new specific delete call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I tried it out. First, I moved the instantBuy
method into the transaction block, but it didn’t work. It seemed that the order created in the transaction block wasn’t fully completed, which caused some database errors in the instantBuy
method. So, I had to implement it this way instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, if you're certain about it.
However, maybe in this case the Order.transaction(){} blocks are no longer needed, just
try{
orderService.create()
}
catch{
orderService.delete()
}
?
Context
Changes